Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Drop couple of deprecated build items and a field in Security and Vert.x HTTP Security area, mark redirect-after-login for removal #46316

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Drop couple of deprecated build items and a field in Security and Vert.x HTTP Security area, mark redirect-after-login for removal #46316

merged 1 commit into from
Feb 20, 2025

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Feb 17, 2025 •
edited
Loading

This PR drops:

  • io.quarkus.vertx.http.runtime.security.ImmutablePathMatcher.PathMatch#remaining was not part of API, I created this inner class with this field already as deprecated from another class because I wanted to be caution back then, IMO noone uses it as it is not reliable anymore
  • io.quarkus.security.spi.AdditionalSecuredClassesBuildItem deprecated since 2.15 because we need to have method-level security (except for a special cases like WS HTTP Upgrade)
  • io.quarkus.vertx.http.deployment.HttpSecurityPolicyBuildItem deprecated since 3.6 but users don't need this item, they can make their policies beans instead
  • replacing usage of public HttpCredentialTransport getCredentialTransport() on HttpAuthenticationMechanism implementors as the method has been marked for removal since 2.8

Configuration property io.quarkus.vertx.http.runtime.FormAuthRuntimeConfig#redirectAfterLogin is now marked for removal. It has been deprecated since 2.16 and users can't see deprecated properties in configuration tables once marked as deprecated.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 17, 2025 •
edited
Loading

🎊 PR Preview d9b444e has been successfully built and deployed to https://quarkus-pr-main-46316-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@michalvavrik michalvavrik force-pushed the feature/drop-security-extension-deprecations branch from f027388 to a67a914 Compare February 17, 2025 21:39
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@sberyozkin
Copy link
Member

sberyozkin commented Feb 17, 2025 •
edited
Loading

LGTM, but I recall receiving some concerns after I removed some deprecated OIDC API code, I have to admit I'm not 100% sure when is the right time for removing such methods.

@maxandersen @gsmet What do you think, can we remove this HttpAuthenticationMechanism method which was deprecated long time ago ?

No concerns for other 3 updates

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@michalvavrik
Copy link
Member Author

michalvavrik commented Feb 18, 2025 •
edited
Loading

LGTM, but I recall receiving some concerns after I removed some deprecated OIDC API code, I have to admit I'm not 100% sure when is the right time for removing such methods.

I could write a build step that if any HttpAuthenticationMechanism implementor with public HttpCredentialTransport getCredentialTransport() is detected, build fails and we can keep it there till after the next LTS. I am not convinced it is necessary, but I'll let you decide, should be easy enough.

@sberyozkin
Copy link
Member

Michal, please don't spend time on it :-), let me ping Thomas directly

@sberyozkin
Copy link
Member

Hi @michalvavrik Can you please move HttpAuthenticationMechanism update to its own dedicated PR for this specific update alone be linkable

@michalvavrik michalvavrik force-pushed the feature/drop-security-extension-deprecations branch from a67a914 to eda2949 Compare February 20, 2025 15:13
@michalvavrik michalvavrik changed the title Drop couple of deprecated build items, method and field in Security and Vert.x HTTP Security area Drop couple of deprecated build items and a field in Security and Vert.x HTTP Security area, mark redirect-after-login for removal Feb 20, 2025
@michalvavrik
Copy link
Member Author

Hi @michalvavrik Can you please move HttpAuthenticationMechanism update to its own dedicated PR for this specific update alone be linkable

I dropped removal of the method, but remaining changes related to HttpAuthenticationMechanism are not breaking anymore.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 20, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit eda2949.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 20, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit eda2949.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/infinispan-cache/deployment

✖ io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls - History

  • expected: "thread1" but was: "thread2" - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: 

expected: "thread1"
 but was: "thread2"
	at io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls(InfinispanCacheTest.java:283)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:513)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:427)

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging/deployment

✖ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-6","-7","-9","-10","-11","-12","-13","-14"] to start with: ["-6", "-7", "-8", "-9"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-6","-7","-9","-10","-11","-12","-13","-14"]
to start with:
  ["-6", "-7", "-8", "-9"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:41)

⚙️ Maven Tests - JDK 17

📦 integration-tests/devmode

✖ io.quarkus.test.devui.DevUIGrpcSmokeTest.testTestService - History

  • Too many recursions, message not returned for id [169886414] - java.lang.RuntimeException
java.lang.RuntimeException: Too many recursions, message not returned for id [169886414]
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:164)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
	at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)

@sberyozkin sberyozkin merged commit e701f48 into quarkusio:main Feb 20, 2025
57 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 20, 2025
@michalvavrik michalvavrik deleted the feature/drop-security-extension-deprecations branch February 20, 2025 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sberyozkin sberyozkin sberyozkin approved these changes

Assignees
No one assigned
Labels
area/amazon-lambda area/security area/vertx triage/flaky-test
Projects
None yet
Milestone
3.21 - main
Development

Successfully merging this pull request may close these issues.
2 participants
@michalvavrik @sberyozkin